-
-
Notifications
You must be signed in to change notification settings - Fork 1.1k
3-state boolean columns emphasis on NOT NULL constraint #363
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you! Minor notes below.
Co-authored-by: Phil Pirozhkov <pirj@users.noreply.github.com>
Hello @koic ! Would you have some time to review this so that we can follow up with the modification of the rubocop cop ? |
@rubocop/style-guide-editors WDYT? |
@@ -1167,19 +1167,32 @@ And you'll have to consider the fact that most non-trivial apps share a database | |||
|
|||
=== 3-state Boolean [[three-state-boolean]] | |||
|
|||
With SQL databases, if a boolean column is not given a default value, it will have three possible values: `true`, `false` and `NULL`. | |||
With SQL databases, if a boolean column is nullable, it will have three possible values: `true`, `false` and `NULL`. | |||
Boolean operators https://en.wikipedia.org/wiki/Three-valued_logic[work in unexpected ways] with `NULL`. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This article is long and math heavy, is there perhaps a better reference?
To avoid such situations, boolean columns should always have a default value and a `NOT NULL` constraint. | ||
To avoid such situations, boolean columns should always have a `NOT NULL` constraint. | ||
|
||
Note that when adding a boolean column to an existing table, a default value should be put in place. Otherwise the `NOT NULL` constraint will break for existing rows. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Or it can be added without the constraint, backfilled, and have the constraint added afterwards.
As discussed in #343, the problem of 3-state boolean columns comes from the nullability of the column.
This PR adds emphasis on this. Default values are only required when adding a column to an existing table so
create_table :users { |t| t.boolean :active, null: false }
is completely valid.fixes #343